feat(settings): first-class remote-core connection setting + live status (GH-4396)#4406
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughAdds a new Core connection settings panel for local/remote core selection, live reachability checks, remote URL/token validation, save-and-restart persistence, settings navigation wiring, tests, and translated UI text. ChangesCore Connection Panel
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CoreConnectionPanel
participant Redux
participant Tauri
participant coreRpcClient
User->>CoreConnectionPanel: open settings panel
CoreConnectionPanel->>Redux: read cloud core URL
CoreConnectionPanel->>Tauri: invoke core_rpc_url
CoreConnectionPanel->>coreRpcClient: testCoreRpcConnection(url)
coreRpcClient-->>CoreConnectionPanel: live status
User->>CoreConnectionPanel: click Save
CoreConnectionPanel->>Redux: update core mode
CoreConnectionPanel->>Tauri: restartApp()
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0918dbc17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <SettingsSwitch | ||
| id="core-use-remote" | ||
| checked={useRemote} | ||
| onCheckedChange={next => { | ||
| setUseRemote(next); |
There was a problem hiding this comment.
Keep local mode unavailable in web builds
In non-Tauri web builds the boot picker intentionally forces cloud mode because the browser cannot invoke start_core_process, but this new settings toggle is rendered unconditionally. If a web user turns it off and saves, the local branch persists openhuman_core_mode=local; after reload BootCheck enters local mode and fails trying to start a Tauri-only core, stranding the user on the unreachable recovery screen. Disable/hide the local option when !isTauriEnvironment() so web stays cloud-only.
Useful? React with 👍 / 👎.
| </label> | ||
| <SettingsTextField | ||
| id="core-remote-token" | ||
| type="text" |
There was a problem hiding this comment.
Mask the persisted remote-core token
When a user already has a remote core configured, this field is prefilled from the persisted coreMode.token and rendered as plain text, so simply opening Settings → Core connection exposes the long-lived OPENHUMAN_CORE_TOKEN during screen sharing or shoulder-surfing. Since that bearer grants access to the remote core, render it hidden by default (for example type="password" with an explicit reveal/replace affordance).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/settings/panels/CoreConnectionPanel.tsx`:
- Around line 101-140: The connection checks in runLiveCheck and the manual test
flow are calling testCoreRpcConnection without an AbortSignal, which can leave
the UI stuck in checking/testing on slow or unresponsive endpoints. Add
cancellation/timeout handling by creating and passing an AbortSignal to
testCoreRpcConnection in both paths, and make sure the signal is aborted/cleaned
up when the check is superseded, completes, or times out so the status can
recover promptly.
- Around line 362-371: The bearer token field in CoreConnectionPanel is
currently rendered as plain text, which exposes a secret during normal use.
Update the SettingsTextField for the core token to use a masked password-style
input instead of type="text", and keep any reveal behavior separate if you add
one later. Locate the field by the core-remote-token id in CoreConnectionPanel
and adjust its input type accordingly.
- Around line 154-160: Reject URLs with embedded credentials in
CoreConnectionPanel’s RPC URL validation by checking the parsed URL from new
URL(normalized) for username/password before accepting it. If credentials are
present, set a form error using a new i18n key such as
settings.core.urlCredentialsNotAllowed, and keep the token field as the only
supported credential path. Update the URL validation/normalization flow around
the existing URL parsing logic and ensure the active URL description cannot
render credential-bearing URLs. Add the new translation key through useT() in
en.ts and every locale file.
- Around line 223-253: The handleSave flow in CoreConnectionPanel leaves the UI
stuck if restartApp() rejects because saving is never cleared and the user sees
no error. Wrap the restartApp() call in handleSave with error handling, reset
saving in the failure path, and surface a localized message via useT() using the
new settings.core.restartFailed key. Also add that key to all locale files so
the error text is translated consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: edfe7b02-db04-4680-b6ac-181478c084aa
📒 Files selected for processing (18)
app/src/components/settings/panels/CoreConnectionPanel.tsxapp/src/components/settings/panels/__tests__/CoreConnectionPanel.test.tsxapp/src/components/settings/settingsRouteElements.tsxapp/src/components/settings/settingsRouteRegistry.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.ts
|
Pushed Frontend Checks (quality, i18n, docs, coverage):
PR Submission Checklist: filled in the two items that are now honestly verifiable (format:check passes; diff-coverage measured locally at 90.3%) plus the validation-run metadata. 0 unchecked items remain. No behaviour changed — these are formatting, a lint-annotation, and added tests only. Not merging; that stays with the maintainer. |
…tus (tinyhumansaiGH-4396) Promote the pre-existing cloud core mode (persisted remote-core RPC URL + bearer token, previously reachable only from the pre-router BootCheckGate picker) into a first-class Settings > Core connection panel, and add a live connect/failure status indicator. - New CoreConnectionPanel: live status dot + Recheck, a 'use remote core' toggle, persisted URL + token fields with Test connection, and a Save & restart action that re-enters the existing BootCheckGate flow. - Reuses existing plumbing (coreModeSlice, configPersistence, testCoreRpcConnection); does not introduce a second remote-core mechanism. - Boot-gate hard-fail/fallback semantics unchanged: the panel only surfaces connection state. - Env-var attach path (OPENHUMAN_CORE_REUSE_EXISTING) left as documented dev-only; not surfaced in the UI. - Token stays on the existing localStorage path (audit U3 keychain migration noted in-code as a known follow-up). - Registry + route wiring; 12 new settings.core.* i18n keys translated across all 13 locales. - Vitest coverage for status rendering, the reveal toggle, unreachable state, and the persist/dispatch/restart save flow.
- Apply Prettier to the new panel, test, route wiring, and locale files (frontend format:check lane was red). - Suppress the intentional set-state-in-effect lint warning on the live-check effect (matches existing convention). - Add Vitest cases (Test connection ok/auth/unreachable, live auth-rejected + non-ok statuses, validation errors, switch-back-to-local save) — raises changed-line coverage on CoreConnectionPanel.tsx to ~90% (was ~67%), over the 80% diff gate.
c3fa32c to
27e8203
Compare
CodeRabbit review on tinyhumansai#4396: - Gate local mode off in non-Tauri web builds: a browser can't start a local core, so the toggle now force-remotes + disables when !isTauriEnvironment(), preventing a persisted core_mode=local that bricks the next boot. - Mask the persisted remote-core token: field is type=password by default with a Show/Hide reveal, so opening Settings no longer exposes the bearer during screen-sharing/shoulder-surfing. - Bound the connection checks: testCoreRpcConnection now runs through a 10s AbortController wrapper (live check + manual Test), so a non-responsive endpoint resolves to "unreachable" instead of hanging in checking/testing. - Reject credentials embedded in the RPC URL (user:pass@host) so a secret can't be persisted + echoed back in the active-URL description. - Recover on restart failure: handleSave wraps restartApp in try/catch and resets `saving` (+ surfaces an error) instead of wedging the Save button. Tests: mock isTauriEnvironment/invoke so desktop-vs-web-build gating is controllable; new test asserts a web build force-disables the local toggle. 12 tests pass; pnpm typecheck clean.
Summary
core.pingagainst the currently-active core, with a Recheck button.coreModeSlice,configPersistence,testCoreRpcConnection) — no second remote-core mechanism introduced.settings.core.*i18n keys translated across all 13 locales.Problem
Pointing the desktop client at a remote
openhuman-corewas only reachable two ways: the pre-router BootCheckGate picker (first launch / "Switch mode"), or shell-level env vars. Normal Windows launch paths (Start Menu shortcut,openhuman://auth/...deep link) cannot carry those env vars, so the env-var attach path silently fell back to spawning the in-process core, and there was no persistent in-app affordance or visible status for a remote core once past the boot gate. See #4396 (split from #2437-A).Scope was agreed as promote the existing cloud mode + surface status, not a greenfield build.
Solution
CoreConnectionPanel.tsx(new): live status indicator (connected remote/local, token-rejected, unreachable, checking) + Recheck; aSettingsSwitchremote toggle; URL + token fields reusing the BootCheckGate cloud-picker validation andtestCoreRpcConnection; and Save & restart which persists viastoreRpcUrl/storeCoreToken/storeCoreMode, dispatchessetCoreMode, clears the RPC caches, and callsrestartApp()so the normal BootCheckGate flow re-runs. Save is gated on a dirty check.coreentry +<Route path="core">.settings.core.*keys added toen.tsand all 13 locales with real translations (parity gate green).Design decisions / tradeoffs:
OPENHUMAN_CORE_REUSE_EXISTING) left as documented dev-only and intentionally not surfaced in the UI.localStoragepath the cloud picker already uses; an in-code comment references security audit U3 as the known OS-keychain migration follow-up (not blocked here).attach?url=&token=import is out of scope (deferred; needs security review — phishing/pivot vector).OPENHUMAN_CORE_URL, which does not exist in the codebase. The real vars areOPENHUMAN_CORE_RPC_URL(set by the core to advertise its own bound port — internal discovery, not a point-at-remote input) and the build-timeVITE_OPENHUMAN_CORE_RPC_URL. The actual user-facing remote-core URL is the persisted cloud-mode URL this panel edits.Submission Checklist
CoreConnectionPanel.test.tsx: status rendering per mode, reveal toggle, unreachable failure path, and the persist/dispatch/restart save flow.CoreConnectionPanel.tsxlocally (11 tests). CIdiff-coverconfirms the merged gate on changed lines.N/A: no matrix feature row maps to this settings panel (UI surface over existing cloud-mode behaviour).## Related—N/A: none.testCoreRpcConnection(mock-friendly).N/A: additive Settings panel, no release-cut surface change.Closes #NNN— see## Related.Impact
coreModeslice +configPersistencekeys.Related
openhuman://attach?url=&token=deep-link import (needs security review); optional dedicated "Advanced" settings nav group.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/GH-4396-remote-core-settingsValidation Run
pnpm --filter openhuman-app format:check— passespnpm typecheck— 0 errors in changed files (one pre-existing unrelated error:rehype-highlightmissing inAgentMessageBubble.tsx, untouched here)CoreConnectionPanel.test.tsx— 11/11 pass;pnpm --filter openhuman-app lint0 errors;pnpm i18n:check0 missing/0 extra across 13 localesValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
coreModeslice,configPersistencekeys, and boot-gate fallback semantics are unchanged.setCoreModedispatch + cache-clear sequence; env-var attach path untouched.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes